Detekt do be detekting

We have to use both ModifierLocalModifierNode and ObserverModifierNode to provide and consume local modifiers + get notified of changes in said modifiers.

It's basically the equivalent of onModifierLocalsUpdated previously used.

You can find a sample of ObserverModifier Node here

I stole this from Compose Foundation's Border impl 😛 since we are invalidating our border modifier manually, we can safely avoid unnecessary invalidations

The importantForBounds is basically if talkback should highlight the component taking into account the set border (AFAIK). It's a useless info semantics-wise in the case of borders

You can check it here

We store the delegate in a property (instead of using an init block) because we need to reference it to call resetPointerInputHandler()

The TrackPressScroller is instantiated only once inside the suspending block, capturing the initial state of the parameters. If those parameters change, the scroller becomes stale, so we must manually reset the handler to restart the block and recreate the scroller

Just to clarify - are focusState.isFocused and focusState.hasFocus equivalent in this context?

My initial assumption was that a focusGroup node can never be focused itself, only its children can. This would mean focusState.isFocused is always false on TrackActivationNode, making updateProvidedValue() always compute parentActivated && false = false and silently breaking the activation chain.

To verify, I put together a minimal reproducer simulating trackWindowActivation providing true from above, with onActivated on a child layout node:

private data object SimulatedWindowActivationElement : ModifierNodeElement<SimulatedWindowActivationNode>() {
    override fun create() = SimulatedWindowActivationNode()
    override fun update(node: SimulatedWindowActivationNode) = Unit
    override fun InspectorInfo.inspectableProperties() { name = "simulatedWindowActivation" }
}

private class SimulatedWindowActivationNode : Modifier.Node(), ModifierLocalModifierNode {
    override val providedValues = modifierLocalMapOf(ModifierLocalActivated to true)
}

fun main(): Unit = application {
    Window(onCloseRequest = ::exitApplication) {
        IntUiTheme(theme = JewelTheme.lightThemeDefinition(), styling = ComponentStyling.default()) {
            var activationReceived by remember { mutableStateOf(false) }

            Column(Modifier.then(SimulatedWindowActivationElement).trackActivation()) {
                Text(
                    "Activation received: $activationReceived",
                    modifier = Modifier.onActivated { activationReceived = it },
                )
                OutlinedButton(onClick = {}) { Text("Click to focus") }
            }
        }
    }
}

I also added logging inside TrackActivationNode.onFocusEvent and updateProvidedValue:

override fun onFocusEvent(focusState: FocusState) {
    println("onFocusEvent: isFocused=${focusState.isFocused} hasFocus=${focusState.hasFocus}")
    println("node.isAttached=$isAttached")
    println("focusState=$focusState")
    println("focusState::class=${focusState::class}")
    if (isFocused != focusState.isFocused) {
        isFocused = focusState.isFocused
        updateProvidedValue()
    }
}

private fun updateProvidedValue() {
    println("updateProvidedValue: parentActivated=$parentActivated isFocused=$isFocused result=${parentActivated && isFocused}")
    provide(ModifierLocalActivated, parentActivated && isFocused)
}

Which produced the following output when clicking the button:

updateProvidedValue: parentActivated=true isFocused=false result=false
onFocusEvent: isFocused=false hasFocus=false
node.isAttached=true
focusState=Inactive
focusState::class=class androidx.compose.ui.focus.FocusStateImpl
onFocusEvent: isFocused=true hasFocus=true
node.isAttached=true
focusState=Active
focusState::class=class androidx.compose.ui.focus.FocusStateImpl
updateProvidedValue: parentActivated=true isFocused=true result=true

activationReceived correctly transitions to true, and isFocused and hasFocus move together. The TrackActivationNode receives FocusStateImpl.Active - not ActiveParent - when a child gains focus, which suggests onFocusEvent delivers the bubbled-up state from the event source rather than reflecting the layout node's own focus state in the tree hierarchy.

Is that the right understanding? If so, focusState.isFocused is correct here and the two properties are equivalent in this context.

tl;dr but isFocused == this node is the one owning the focus, hasFocus == this node or any of its children have the focus

In this case we could use focusState.isFocused or focusState.hasFocus interchangeably 😄

Given the nature of the code (active if the parent or children have focus), hasFocus is more semantically aligned with the intent, though.

Mine understanding was that the node focusGroup is attached to can't itself be focused but only it's children. That why I though focusState.isFocus would not be true, and focusState.hasFocus is the API we had to use here :)
Thanks for explaining this!
@DanielSouzaBertoldi I agree with you that usage of hasFocus is more semantically aligned and it's helps reader understand the code easier. 👍

Shouldn't this be using hasFocus instead of isFocused, as discussed? Or was there a reason why it wasn't switched?

Copy-paste mistake 👯 trackWindowActivation is listed twice.

Nit: Could we add the @Stable annotation here? The other public modifier functions in this file gained it in this PR, and it seems correct to add it here as well for consistency.

Nit: Should this be private val? Every other node uses a private listener.

Nit: I'd rather this be private val to keep it properly encapsulated inside the node.

Nit: typo

Shouldn't this be using hasFocus instead of isFocused, as discussed? Or was there a reason why it wasn't switched?